fix: Keep overrides through an in-place rewrite of their files - #560

Summary

Applications and editors rewrite data files in place: they truncate the file and then write the new content. A reload that reads the file in between finds it empty. On this stack, with pyyaml installed (it is in the test-filesource extra), such a reload applied an empty document whatever the file's name, so the override source briefly removed every override until the next reload. A client evaluating an overridden flag during a slow rewrite (truncate, pause 300 ms, write) served a -> default -> b: on every rewrite in watching mode, and whenever a poll landed in the pause in polling mode (2 of 10 YAML rewrites, 2 of 20 JSON rewrites). Without pyyaml, an empty JSON file failed to parse, the overrides stayed, and the failure was logged at Error level.

The OVERRIDE spec's retry requirement exists to keep "a partially written or briefly invalid file from emptying the layer" (section 1.3). This change makes the file-based override source keep its overrides through such a rewrite. The legacy file data sources (Files.new_data_source, Files.new_data_source_v2) have their own code and are not changed; they never accepted an empty file and serve a -> b for the same rewrite.

The format is chosen by the file's extension

The parser was chosen the way the file data sources choose it: the YAML parser for every file when pyyaml is installed, the JSON parser for every file otherwise. So whether an empty file was valid depended on an installed package, not on the file. The OVERRIDE spec (1.3.3, sdk-specs #272) recommends choosing the parser by extension, lets an SDK whose file data source inspects the content keep doing so, and says nothing about empty files. This code has not been released, so it now chooses by extension and lets the format decide: a file whose name ends in .yaml or .yml, in any case, is YAML, including a file named just .yaml, and any other file, including one with no extension, is JSON. Whether an empty file is valid is then up to the format, like the rest of the content.

• A JSON file is parsed by the standard JSON parser even when pyyaml is installed, which reads some content differently from the YAML parser it replaces:
  • YAML content in a .json file no longer loads, nor does JSON that only YAML accepts (a tab inside a string).
  • A JSON document must be an object: null, an array, a number, a string or a boolean fails to parse, as in the other SDKs. YAML null still has no entries.
  • 1e3 is now a number (YAML 1.1 read it as the string "1e3"), and NaN, Infinity and -Infinity are accepted as numbers, as Python's json module does.
  • A parse error says which format the file's name chose (error parsing file as JSON, the format of its name: ...).
• A byte order mark at the start is ignored, as the YAML parser already did, so a .json file with one keeps loading.
• A YAML file without pyyaml is a configuration error: FileOverrideSourceBuilder.build raises ValueError, as it does for watching mode without watchdog (OVERRIDE 1.2.9, 1.3.3). Before, such a file was parsed as JSON on every reload.

I checked the rest of the engine for other rules that look at content instead of the extension, and found none: a missing file is skipped before parsing, and YAML null is read as nothing, like an empty YAML file.

An empty JSON file is invalid

An empty JSON file, or one with only white space, now fails to parse with the file has no content, which is not a JSON document, and the overrides in effect stay, as for any other content that cannot be parsed. An empty YAML file is still a document with no entries. To remove every override from a file, write {} or delete the file.

A file found with no entries is held back until it is written

With the JSON rule alone, a JSON file that is empty for a moment fails to parse and is retried, but the failure is logged at Error level for an ordinary write. A YAML file can legitimately be empty, so its parser cannot tell an emptied file from one in the middle of a rewrite. The Reloader now decides, for every load, whether a file may be in the middle of a rewrite, whatever its format.

A file has no entries when it is empty (white space counts as empty) or when its parser reads it as nothing: YAML with only a comment or a --- marker, which a read can see of a YAML file that is being written. {} is a document, written on purpose, and is applied right away. When a load finds a file with no entries while that file's content is in effect, it holds the file back: the current data stays, the load logs at Debug level and reports nothing, and the other files' changes wait too, since a load applies all files or none. The file is held back until it has stayed so, without being written, for the retry delay (1 second). The hold arms the retry for the moment the first held file is due, its delay or its bound being over, plus a tenth of the retry delay, and the write of the new content changes the file, so its change signal starts a load sooner, which applies the new content. After the delay, a YAML file is applied with no entries, and an empty JSON file fails to parse and keeps the overrides.

The Reloader keeps:

• for each file, whether it contributed content to the data in effect, updated when a load is applied and when one is skipped as unchanged. The skip-unchanged digest covers each path, whether its file exists, and its bytes: the format comes from the extension, so the same bytes moved from x.yaml to x.json parse differently and are not skipped, and renaming a file to another configured path (mv a.yaml b.yaml) is applied, so the record follows the content;
• for each file found with no entries while its content is in effect: since when it has had none without being written (cleared when a load reads it with content, even content that fails to parse, or finds it missing), its state then (modification time and size, read through symbolic links with the poller's own helper), and when a load first held it back, which bounds the hold. The bound starts over only at a load that holds nothing back, for every file that load got to (with a document, parsed empty, found missing, or failing to be read or parsed), whether the load applies, is skipped or fails; every file is reached, since a load reads all of them even after a failure. Applying data, or a load skipped as unchanged, forgets every file's entry.

So:

• A change of a held back file's state means it was written since a load last found it so, for example rewritten and truncated again between two loads, so its delay starts over. States are only compared with each other, never with the clock.
• A file is held back for at most five retry delays after a load first held it back, so a file that keeps being written while every load finds it empty, or files rewritten in turn so that every load finds one of them empty, are applied in the end. The bound is per file: a file that reaches its bound does not cut short another file's ordinary rewrite. It starts over only when no file is held back, so files emptied in staggered turns, each settling while the other holds the load, are still applied in the end, another file failing to parse every few seconds does not start it over, and a file held during a long failure of another file is bounded afresh once a load reads it without holding anything back.
• The clock is read after each file's own read, and again for the retry the hold arms, so a slow read of another file does not shorten or lengthen a file's delay.
• A further change signal, or a retry left over from an earlier failure, while the file is still held back holds it back too.
• Every file is read, parsed and classified, even after another file fails, before the load fails, holds files back, or merges, in that order. So files emptied together wait together, one retry delay rather than one per file; a file that cannot be read or parsed fails the load with the first failure, reported at once, even while another is held back; and every file is reached, so a load that holds nothing back starts every bound over, including that of a file configured after the failing one.
• A file whose content is not in effect is never held back: a placeholder YAML file that has always been empty, a file that is deleted and then created empty, and a file whose emptiness has been applied. Deleting a file that had content still removes its overrides at once.
• A hold is not a failure: nothing is reported, and the next success is not treated as a recovery, so rewriting a file in place with the same content applies nothing.
• With retries off (retry_delay=0), nothing would load the file later, so an empty file is parsed right away. The override source always retries.

A rewrite is covered when the gap between truncating and writing is shorter than the retry delay. When a writer pauses longer, the retry finds the file still empty: a YAML file is applied with no entries until the write, and a JSON file is reported as a failure and keeps the overrides.

To decide before parsing, a load reads each file (_read_file), parses it (_parse_file, which tells a document from nothing), classifies it, and only after the last file merges them (_merge_files); one read still feeds both the skip-unchanged digest and the parse. The hold is timed by an injectable monotonic clock, read after each file's own read, which the deterministic tests replace, and the retry each load asks for is recorded for the tests.

This is the eighth version of the rule, the same as the current draft of dotnet-core #394. The first held a file back once, when its previous read had found content; an adversarial review found three sequences that still apply an empty YAML file (two quick rewrites in one debounce window, a change to another file while one is mid-rewrite, a polled file rewritten about a second apart). The second keyed the delay on the file's modification time with a per-file cap; reviews of the other SDKs found that files rewritten in turn were held without end, that a slack in the delay check let another load apply a gap shorter than the delay, and that YAML with only a comment was applied as no entries. The fourth, the first commit of this PR, bounded the run of loads that hold files back; a review of .NET found that a failing load restarted that bound, that one file reaching it cut short another file's rewrite, and that a clock read once per load mismeasured a file read after a slow read. The fifth bounded each file on its own but started a file's bound over when it settled or was read with content; a review found that files emptied in staggered turns were then held without end, and that a bound kept through a long failure applied a later rewrite empty. The sixth did not start over the bound of the file whose read or parse failed, so the in-place save that fixes a typo, saved in place too, was applied empty. The seventh stopped a load at the first failure, so a file configured after the failing file kept a stale bound, and its next rewrite was applied empty (found by the Java and Ruby reviews). Each of those cases has a test here, and each fails with the earlier rule it broke.

Documentation

The public docs say only what an application needs; how the hold works is in the internal comments and in this description.

• FileOverrideSourceBuilder says that the file name decides the format (.yaml or .yml is YAML, which needs pyyaml; anything else is JSON) and that files must be UTF-8. It says that files can be rewritten in place, and that the source keeps the overrides in effect while it finds a file empty during a rewrite. It says how to remove a file's overrides (delete the file or write {}), that an empty JSON file is not a valid document, and that a partly written YAML file can still parse, so a YAML file is best replaced in one operation.
• build lists the new configuration error.
• Internal: the Reloader, _FileOverrideSource and parse_document docstrings and comments describe the hold, its delay and bound, the retry it asks for, and the empty-file rules.

Not changed: YAML cut off part way

A read that overlaps a write can see the first part of a YAML file, and YAML cut off at almost any point can still parse: with fewer entries, or with a different value (flagValues:\n flag1: gives a null variation; flag2: 10 cut from flag2: 100 gives 10). Such a part is applied until the next reload. JSON cut off anywhere fails to parse. This predates this change. Telling a part from a whole file needs a content rule this change does not add, so the docs recommend replacing a YAML file in one operation.

Difference from the other SDKs

.NET (dotnet-core #394) makes the same change. Go still detects the format from the content, so an empty file of any name loads as no overrides there. The legacy file data sources in this SDK still parse every file with the YAML parser when pyyaml is installed, so a .json file that only YAML accepts loads there but not in the override source.

Testing

Python CI runs only for pull requests based on main or feat/**, so this stacked PR gets no CI. Everything below ran locally on Python 3.13 with all extras (pyyaml 6.0.3, watchdog 7.0.0).

• Parser: the extension decides (.yaml, .yml in any case, also a file named just .yaml; anything else is JSON, also with pyyaml installed); empty, white-space and byte-order-mark-only JSON fails with the message above; JSON that is not an object (null, an array, a number, a string, a boolean) fails; YAML null and ~ have no entries; empty, white-space, --- and comment-only YAML has no entries; a byte order mark is ignored; YAML without pyyaml fails; each file of a load is parsed in its own format.
• Builder: a .yaml or .YML path without pyyaml raises ValueError; JSON paths do not.
• Reloader tests on a fake clock (a retry delay of an hour, so no real retry fires, and every load started with reload_now):
  • A file is held back until it has stayed empty for exactly the retry delay: a load at 95 % of it still holds.
  • A write of the held back file, seen as a change of its state, starts the delay over; also through a symbolic link.
  • Two quick rewrites: the file is written and truncated again before a load reads the new content; it stays held back, and the content finally written is applied at once.
  • White space only, a comment only, --- only and YAML null count as no entries, and so does a JSON file holding only a byte order mark, which is held back quietly; {} is applied at once; a JSON file holding null is reported as not an object.
  • Two files emptied together are applied together after one retry delay.
  • A change to one file waits while the other is held back.
  • A file that cannot be parsed is reported at once while another file is held back.
  • The bound: a file kept written, with a new modification time at every load, is applied after five retry delays; so are two files rewritten in turn. Another file failing every other load does not start the bound over, and a file reaching its bound does not apply another file that is a quarter of a delay into a rewrite. Two files emptied in staggered turns (one re-truncated every 0.9 delays, with a retry load after each) are applied within 14 turns. A file held back while another file fails, then read with content, is held back afresh six retry delays later when the other file is fixed in the same load. A file saved in place with a typo and, six retry delays later, with the fix (YAML and JSON) has the fix's truncation held back, without another report. A file kept written while empty is applied after its bound while an earlier file fails, and loads that hold a file back and then fail do not start bounds over. A file after one that fails to parse, or that cannot be read, is still read, so its bound starts over, and its rewrite six retry delays later is held back. A load with two broken files reports the first, also when the second is an empty JSON file, and logs the others at Debug level. A file whose read fails (an unreadable file) is counted as read, so its bound starts over and its next emptying is held back, and it keeps its delay, since a failed read says nothing about the content. A file emptied, deleted, and created empty again with the first emptying's modification time starts a new delay.
  • A slow read of an earlier file (the fake clock advanced inside the read) does not shorten a later file's delay, and a slow read of a later file does not lengthen an earlier file's.
  • The retry each hold asks for: the delay and a tenth after the first hold, the rest of the delay after a later one, the bound cutting it short, the first file due deciding (in either order), only the margin when a slow read of a later file takes the held file past its delay, the same through a change signal, and a failure asking for the retry delay.
  • A file emptied again long after its content was applied is held back again, and so is one whose modification time is set back to that of the first emptying, as a coarse file system can make it look, whether or not data was applied in between.
  • A JSON file that stayed empty for long, and failed, is held back again without another report when it is rewritten in place.
  • A file rewritten in place with the same content, so that the load is skipped as unchanged, and rewritten again six retry delays later, is held back again: the skip starts the bounds over like an application.
  • The same bytes moved from x.yaml to x.json (1e3, a string in YAML and a number in JSON) are applied, not skipped as unchanged.
  • UTF-16, UTF-32 and invalid UTF-8 files fail to parse, as JSON and as YAML, and the reloader reports such a file and keeps loading.
  • Placeholder, deleted-then-created-empty, content moved away from a file and content moved to it, retries off (YAML applied, JSON reported), a same-content rewrite that applies and reports nothing, and the initial loads of empty YAML and JSON.
• Reloader tests on the real clock, waiting for the Debug line rather than sleeping: the retry after a hold logs Reading flag data again after a file was found empty, and the retry after a failure Retrying flag data load after earlier failure; a held back file applied by the load its write triggers; a file that stays empty applied by the first retry the hold armed, one retry delay after it was found empty; a further hold that does not put off that retry; an empty JSON file reported only at the retry; the retry of a first rewrite finding the second truncation; a retry left over from a failure holding the file back; a retry left over from a hold applying nothing after a load on demand.
• End to end, with real in-place rewrites (truncate, pause, write; rewrite_in_place in ldclient/testing/file_util.py), for JSON and YAML, in watching and polling modes:
  • The override source never supplies a snapshot without the override, and the retry the empty file armed supplies nothing more.
  • A client evaluating the flag throughout serves a -> b, never the default.
  • In watching mode the pause is 300 ms. In polling mode it is 1.6 s, longer than the 1 s poll interval, so that a poll always finds the file empty; the tests set a retry delay longer than the pause (1.5 s and 3 s) so the rewrite completes before the retry on a loaded machine.
  • Successive rewrites with the source's own timing (1 s retry delay, 1 s poll interval, 300 ms pauses): back to back when watching, about a second apart when polling. No snapshot lacks the override.
  • An emptied JSON file keeps the overrides and logs the failure, and {} then removes them; an emptied YAML file removes them after the retry delay.
• The Reloader tests now rewrite their file in place with a 30 ms pause after truncating, so they exercise this case deliberately instead of by chance. test_reloader_reports_identical_failure_only_once now also checks that each report is the one it expects or the empty file, and that nothing is applied.
• A scratch client that evaluates the flag every millisecond during 12 slow in-place rewrites (300 ms pause, at random points of the poll interval) serves only a -> b in all six combinations (JSON and YAML, watching and polling, and JSON without pyyaml), with no Error lines. On the base it served the default on every rewrite when watching, and an empty JSON file without pyyaml logged an Error on every rewrite.
• On the base of this branch, 89 of the 235 tests in test_filedata.py (176) and test_file_override_source.py (59) fail, with a shim that adds the new helper names and accepts the test clock so that each failure is about behavior. All the end-to-end rewrite tests fail there. Each earlier version of the rule fails some of the test_filedata.py tests: 16 (first), 7 (second), 9 (fourth, the first commit of this PR), 6 (fifth), 4 (sixth), 5 (seventh, the second commit).
• Mutation check on the final code: 59 mutants, each reverting or bending one part of the change, each run against both test files. 54 fail at least one test. The five that survive are equivalent here: the explicit "retries off" check (with a zero retry delay no file can be held anyway); remembering what each file contributed on a load skipped as unchanged, and forgetting the held-file records when data is applied or a load is skipped (the digest tells the files apart, so a skipped load has the same per-file content, and a load that holds nothing back already starts every bound over); and leaving the paths out of the digest (the configured paths are fixed, so the position and presence of each file already tell a move apart; leaving out both paths and presence fails the move tests). They are kept to match the other SDKs and to keep the digest's meaning plain.
• Loops: both test files 10 times in a row, 0 failures (176 and 59 tests per run).
• Full unit suite (LD_SKIP_DATABASE_TESTS=1 pytest -W error::SyntaxWarning): 2090 passed, 367 skipped. mypy ldclient, isort --check, pycodestyle and the Sphinx docs build (-W -E) pass.
• Contract tests: the sync and async test services pass the harness's flag overrides suite (79 run, 15 skipped by capability), including its change notification tests.

_{Opened from termag}